Skip to content

Feature flags v1: schema, evaluator, routes, dashboard#1392

Draft
mantrakp04 wants to merge 8 commits intodevfrom
feature-flags-v1
Draft

Feature flags v1: schema, evaluator, routes, dashboard#1392
mantrakp04 wants to merge 8 commits intodevfrom
feature-flags-v1

Conversation

@mantrakp04
Copy link
Copy Markdown
Collaborator

@mantrakp04 mantrakp04 commented Apr 28, 2026

Summary

First-cut feature flag system: definitions in branch config, a pure deterministic evaluator shared between backend and SDK-facing types, bootstrap/evaluate routes, dashboard CRUD, SDK helpers, seed data, and an example demo page.

This also includes the follow-up review fixes around evaluator correctness, config validation, client-side context spoofing, stable bootstrap caching, dashboard save behavior, and SDK helper ergonomics.

Walkthrough

Data model — packages/stack-shared/src/config/schema.ts
branchFeatureFlagsSchema plugs into branchConfigSchema as featureFlags. Definitions live in config so they inherit branch/environment overrides for free; evaluation data such as exposures, audit logs, cohorts, and experiment events can live in dedicated tables later.

Each flag has an opaque config id plus a user-facing key, a type (boolean | multivariate | json | numeric | string), variants, priority-ordered rules, defaultVariantKey, and operational controls like enabled, killSwitch, dependsOn, and holdoutId.

Validation now enforces:

  • rule variant selection uses exactly one of variantKey or variantWeights
  • flag keys are unique when flags are present
  • regex conditions are valid and bounded before config can save
  • defaultVariantKey, rules.*.variantKey, and rules.*.variantWeights only reference existing variants

Evaluator — packages/stack-shared/src/feature-flags/evaluator.ts
Pure, no IO. evaluateFlag(flagId, config, ctx) returns { variantKey, value, reason, ruleId? }.

Resolution order is: missing -> kill_switch -> disabled -> dep_unmet/cycle -> holdout -> matched_rule -> default.

Rules sort by priority descending with id tiebreaks. Conditions support eq/neq/contains/regex/gt/in/before/semver_*/in_cohort/... over dotted attribute paths like user.email, team.plan, or context.country.

Review fixes here:

  • dependsOn uses shared truthiness, including non-empty JSON objects/arrays
  • semver_* comparisons now handle prerelease precedence and ignore build metadata
  • regex patterns are validated, cached, and matched against bounded-length strings
  • dynamic result dictionaries use Map internally before serialization

Hashing — packages/stack-shared/src/feature-flags/hashing.ts
MurmurHash3 32-bit. Salt convention is ${flagKey}.${ruleId}.${rolloutSeed} so the same user can land in different buckets across flags.

The bucket separator is now a named "\\x01" constant instead of an invisible literal control character. That keeps assignments stable while making the hashing contract visible in review/diffs.

Routes — apps/backend/src/app/api/latest/feature-flags/

  • GET /bootstrap returns full flag config for the tenancy. SDKs can cache and evaluate locally. ownerUserId is stripped. The response includes a stable murmur3 version, an ETag, and supports If-None-Match revalidation with 304.
  • POST /evaluate performs server-side eval. distinct_id falls back to the authenticated user id.

Important auth behavior: client-key requests no longer trust caller-supplied targeting context (body.user, body.team, body.context, body.cohorts). Browser clients can still pass distinct_id when using the raw endpoint, but the SDK helpers use the authenticated Stack user id. Arbitrary targeting context is only accepted from server/admin access. This prevents a browser caller from spoofing user.email to match internal targeting rules.

apps/backend/src/route-handlers/smart-response.tsx was also adjusted so 204/304 responses are emitted without bodies, which is required by HTTP and by the platform Response constructor.

SDK helpers — packages/template + sdks/spec
The client SDK now has app methods and provider hooks for evaluate-by-key:

  • app.getFeatureFlag(key)
  • app.getFeatureFlags(keys)
  • app.useFeatureFlag(key)
  • app.useFeatureFlags(keys)
  • top-level React shorthands useFeatureFlag() and useFeatureFlags()

The helpers call /api/v1/feature-flags/evaluate, return camelCase result objects, batch multiple keys, and only send flag_keys. They intentionally do not accept arbitrary client targeting context or caller-provided bucketing ids, matching the route-level anti-spoofing behavior. Bucketing comes from the authenticated Stack user id, including Stack anonymous users when the app creates one.

sdks/spec/src/apps/client-app.spec.md documents the helper contract so future SDK implementations match the JS SDK behavior.

Dashboard + seed
New "Feature Flags" app under /projects/[projectId]/feature-flags, with list + detail editor going through useUpdateConfig rather than a separate API.

Dashboard review fixes:

  • save/delete success toasts and redirects only happen after updateConfig() returns success
  • non-boolean flags start disabled until variants/defaults are configured
  • async button/switch errors are surfaced by the existing async component wrappers

Seed adds three demo flags:

  • new-checkout — 25% boolean rollout
  • pricing-experiment — 50/25/25 multivariate split
  • internal-tools — anchored @stack-auth.com$ email rule for trusted/server-side context

Demo page
examples/demo/src/app/feature-flags-demo/page.tsx validates required public env vars up front and strips trailing slashes from NEXT_PUBLIC_STACK_API_URL, so setup mistakes fail with clear errors instead of producing undefined/api/... requests.

API / SDK usage

Client-side SDK use:

const checkout = await app.getFeatureFlag("new-checkout");

if (checkout.value === true) {
  // ship it
}

React hook use:

const flags = useFeatureFlags(["new-checkout", "pricing-experiment"]);

if (flags["new-checkout"]?.value === true) {
  // ship it
}

Trusted server/admin callers can still use the raw evaluate endpoint with richer targeting context:

await fetch(`${apiUrl}/api/latest/feature-flags/evaluate`, {
  method: "POST",
  headers: {
    "content-type": "application/json",
    "x-stack-access-type": "server",
    "x-stack-project-id": projectId,
    "x-stack-secret-server-key": secretServerKey,
  },
  body: JSON.stringify({
    distinct_id: userId,
    user_id: userId,
    user: { email: primaryEmail },
    context: { country: "US" },
    flag_keys: ["internal-tools"],
  }),
});

Notes:

  • Evaluate by user-facing key, not config id. The route resolves via findFlagIdByKey.
  • SDK helpers bucket by the authenticated Stack user id. Apps that need anonymous bucketing should create/use Stack anonymous users first.
  • Client-supplied targeting attributes are intentionally ignored for client-key requests.
  • reason comes back on every result for debugging and future exposure analytics.
  • /bootstrap is wired up but no SDK consumes it yet; a future SDK can use it for local evaluation and cache revalidation.

Natural follow-up out of scope here: a Vercel Flags adapter that bootstraps at edge time. The evaluator + hashing are designed so local SDK evaluation matches server evaluation.

Test plan

  • pnpm typecheck
  • pnpm test run packages/stack-shared/src/feature-flags/evaluator.ts
  • pnpm test run apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts
  • pnpm test run packages/stack-shared/src/interface/client-interface.test.ts
  • pnpm -C packages/template typecheck
  • pnpm -C packages/stack-shared typecheck
  • pnpm -C packages/template lint
  • pnpm -C packages/stack-shared lint
  • Package lints for backend/dashboard/stack-shared/e2e/demo
  • Verify bootstrap response strips ownerUserId
  • Verify bootstrap ETag / If-None-Match revalidation returns 304
  • Verify distinct_id omission falls back to authenticated user id
  • Verify client-key requests cannot spoof targeting attributes

Dashboard page for manual smoke testing: http://localhost:8101/projects/-selector-/feature-flags

Summary by CodeRabbit

  • New Features

    • Feature Flags system: dashboard UI to create, edit, evaluate, and manage flags (variants, rules, rollouts, holdouts, dependencies) with seeded demo flags and a demo page.
    • Server endpoints for bootstrapping and deterministic evaluation with stable versions/ETag revalidation.
    • Client SDK APIs and React hooks for fetching/evaluating flags.
  • Tests

    • Comprehensive unit and end-to-end tests covering evaluator logic, hashing/bucketing, API behavior, and client integrations.

Definitions live in branch config (free branch/env overrides). Pure
deterministic evaluator shared by backend and (future) SDKs, MurmurHash3
bucketing, kill switch, dependsOn, holdouts, weighted multivariate.
Adds /feature-flags/{bootstrap,evaluate} routes, dashboard CRUD, seed
data, and an example demo page.
Copilot AI review requested due to automatic review settings April 28, 2026 00:39
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
stack-auth-hosted-components Ready Ready Preview, Comment Apr 30, 2026 11:03pm
stack-backend Ready Ready Preview, Comment Apr 30, 2026 11:03pm
stack-dashboard Ready Ready Preview, Comment Apr 30, 2026 11:03pm
stack-demo Ready Ready Preview, Comment Apr 30, 2026 11:03pm
stack-docs Ready Ready Preview, Comment Apr 30, 2026 11:03pm
stack-preview-backend Ready Ready Preview, Comment Apr 30, 2026 11:03pm
stack-preview-dashboard Ready Ready Preview, Comment Apr 30, 2026 11:03pm

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a feature-flag system: runtime-safe types, hashing, deterministic evaluator, new backend bootstrap/evaluate endpoints, dashboard UI for listing/creating/editing flags, client SDK/hooks and client-app integrations, seed/demo data, and tests.

Changes

Cohort / File(s) Summary
Core types & hashing
packages/stack-shared/src/feature-flags/types.ts, packages/stack-shared/src/feature-flags/hashing.ts
Adds runtime-safe feature-flag types, regex helpers, Murmur3-based hashing, bucket and weightedVariant, and unit tests for determinism and distributions.
Evaluator logic & tests
packages/stack-shared/src/feature-flags/evaluator.ts
New pure evaluator with kill-switch, dependsOn (cycle-safe), holdouts, rule operators, rollouts/stickiness, weighted selection, batch helpers, and extensive Vitest coverage.
Config schema & fuzzer
packages/stack-shared/src/config/schema.ts, packages/stack-shared/src/config/schema-fuzzer.test.ts
Adds branchFeatureFlagsSchema, validation rules (unique keys, safe-regex checks, variant/weight constraints) and fuzzer sample data.
Backend API & response handling
apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts, apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts, apps/backend/src/route-handlers/smart-response.tsx
New bootstrap (ETag/versioned payload) and evaluate endpoints; smart-response omits body for 204/304 responses and evaluate returns stable JSON results.
Dashboard UI
apps/dashboard/src/app/(main)/(protected)/projects/.../feature-flags/page-client.tsx, .../page.tsx, .../[flagId]/page-client.tsx, .../[flagId]/page.tsx, apps/dashboard/src/lib/apps-frontend.tsx
Client pages for listing, creating, editing, deleting flags; JSON editors, live evaluate-as-user preview, and app registry entry with Flag icon.
Seed & demo
apps/backend/prisma/seed.ts, examples/demo/src/app/feature-flags-demo/page.tsx
Seeds demo flags into internal project overrides (demo feature flags) and adds a demo client page that calls the evaluate API.
Client SDK & app integrations
packages/stack-shared/src/interface/crud/feature-flags.ts, packages/stack-shared/src/interface/client-interface.ts, packages/template/src/lib/hooks.tsx, packages/template/src/lib/stack-app/...
New request/response types, evaluateFeatureFlags client API, StackClientApp methods/hooks (getFeatureFlag(s), useFeatureFlag(s)), template exports and tests.
E2E & client tests
apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts, packages/template/src/lib/stack-app/.../*.test.tsx
E2E/unit tests covering evaluate/bootstrap behavior, rule precedence, holdouts/cohorts, determinism, and client-side batching/caching.
Docs & registry
docs/src/components/mdx/app-card.tsx, packages/stack-shared/src/apps/apps-config.ts, sdks/spec/src/apps/client-app.spec.md
Adds Flag icon mapping, registers feature-flags in ALL_APPS, and documents client methods/spec for getFeatureFlag(s).
Misc & infra
apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx, packages/stack-shared/package.json, packages/template/src/index.ts
Enforces config validation before project-level overrides, adds safe-regex2 dependency, and re-exports feature-flag hooks from template package.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client/SDK/Dashboard
    participant API as Backend API
    participant Config as Resolved Config Store
    participant Evaluator as Evaluator
    participant Hash as Bucketing

    Client->>API: POST /feature-flags/evaluate { flag_keys?, distinct_id?, context? }
    API->>Config: resolve tenancy.config.featureFlags
    API->>Evaluator: evaluateFlag(flagId, config, context)
    Evaluator->>Evaluator: check enabled / killSwitch / dependsOn
    Evaluator->>Hash: compute holdout bucket (if configured)
    Hash-->>Evaluator: holdout decision
    Evaluator->>Evaluator: evaluate rules (conditions, priority)
    Evaluator->>Hash: rollout & weightedVariant buckets (stickyBy/distinctId)
    Hash-->>Evaluator: selected variant
    Evaluator-->>API: result { flag_key, variant_key, value, reason, rule_id? }
    API-->>Client: 200 JSON results (or 304 for bootstrap if ETag matches)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • BilalG1
  • N2D4

"🐰
I hop through keys and seeded ground,
Buckets hum a murmur3 sound,
Rules align and variants dance,
Holdouts pause a fleeting chance,
Flags deployed—deterministic prance!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: a comprehensive v1 feature flags system implementation including schema, evaluator, routes, and dashboard.
Description check ✅ Passed The description is comprehensive and detailed, covering data model, evaluator design, hashing, routes, SDK helpers, dashboard, seed data, and test plan with clear explanations and code examples.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature-flags-v1

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements a first-cut, config-backed feature flag system with a shared deterministic evaluator, backend API endpoints for evaluation/bootstrap, dashboard CRUD UI, seed demo flags, and accompanying tests/demo documentation updates.

Changes:

  • Added shared feature-flag config types, hashing, and evaluator logic to @stackframe/stack-shared.
  • Introduced backend routes for /api/latest/feature-flags/evaluate and /bootstrap, plus E2E coverage.
  • Added a new Dashboard “Feature Flags” app with list/detail editing, and seeded demo flags + a demo page.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/stack-shared/src/interface/crud/feature-flags.ts Re-exports public feature-flag types/operators for consumers.
packages/stack-shared/src/feature-flags/types.ts Adds runtime TS types for feature flag config and evaluation results.
packages/stack-shared/src/feature-flags/hashing.ts Adds deterministic Murmur3-based bucketing and weighted variant selection.
packages/stack-shared/src/feature-flags/evaluator.ts Adds pure evaluator + helpers (rule matching, dependencies, holdouts, key→id lookup).
packages/stack-shared/src/config/schema.ts Extends branch config schema/defaults to include feature flag definitions and validation.
packages/stack-shared/src/config/schema-fuzzer.test.ts Extends schema fuzzing inputs to cover the new featureFlags config surface.
packages/stack-shared/src/apps/apps-config.ts Registers new “feature-flags” app metadata in shared app registry.
examples/demo/src/app/feature-flags-demo/page.tsx Adds demo page that calls the evaluate endpoint with a publishable key.
docs/src/components/mdx/app-card.tsx Adds icon mapping for the new “feature-flags” app in docs UI.
apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts Adds E2E tests for evaluate + bootstrap and local-vs-server evaluator parity.
apps/dashboard/src/lib/apps-frontend.tsx Adds Dashboard navigation entry for “Feature Flags”.
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/page.tsx Adds route entrypoint for feature flags list page.
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/page-client.tsx Implements flag list + create dialog wiring via config updates.
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/[flagId]/page.tsx Adds route entrypoint for feature flag detail page.
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/[flagId]/page-client.tsx Implements detail editor (metadata/variants/rules JSON) + local debug evaluator.
apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts Adds server-side evaluation endpoint with key→id resolution and result shaping.
apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts Adds bootstrap endpoint returning definitions + key→id map + content version.
apps/backend/prisma/seed.ts Seeds demo feature flags into the internal project for first-run experience.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/stack-shared/src/feature-flags/hashing.ts Outdated
Comment thread examples/demo/src/app/feature-flags-demo/page.tsx
Comment thread packages/stack-shared/src/config/schema.ts
Comment thread packages/stack-shared/src/feature-flags/evaluator.ts Outdated
Comment thread apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 28, 2026

Greptile Summary

This PR introduces a complete feature flag system: a config-schema-backed definition model, a pure deterministic evaluator shared by the backend and SDK, bootstrap/evaluate routes with ETag caching, a dashboard CRUD UI, and client SDK helpers with React hooks. The architecture is well-thought-out — client-key context spoofing is blocked, regex patterns are validated and cached with a ReDoS guard, the MurmurHash3 bucketing is stable, and cross-reference validation (dependsOn, holdoutId, variantKey) is enforced at save time.

  • P1 — not_in/in conditions silently misbehave with non-array values: value: yupMixed() in the condition schema allows any JSON type for all operators. When not_in receives a plain string instead of an array, applyOperator returns true for every user (always matches), turning an exclusion rule into an unconditional serve-all rule with no save-time error.

Confidence Score: 3/5

Safe to merge with caution — the not_in/in operator footgun can silently produce wrong flag evaluations for misconfigured rules.

One P1 finding (in/not_in operator value type not validated in schema) can cause silent wrong targeting for any rule that uses those operators with a non-array value. The PATCH non-atomic read-modify-write is a pre-existing pattern concern. Core evaluator, hashing, auth controls, and dashboard flow are solid.

packages/stack-shared/src/config/schema.ts — condition value type validation for in/not_in operators; apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx — PATCH atomicity.

Important Files Changed

Filename Overview
packages/stack-shared/src/feature-flags/evaluator.ts Pure deterministic flag evaluator with LRU regex cache, safe-regex guard, correct semver prerelease comparison, cycle detection, and comprehensive in-source tests. No issues found.
packages/stack-shared/src/feature-flags/hashing.ts MurmurHash3 bucketing with named separator constant and reference vector tests. Implementation is correct and stable.
apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts Evaluate endpoint with client-key context stripping, Map-based results, and graceful missing-flag handling. Previously flagged spoofing and proto-pollution issues are resolved.
apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts Bootstrap GET with ETag/304 revalidation, ownerUserId stripping, and murmur3 content hash. Parsing logic is correct.
packages/stack-shared/src/config/schema.ts Feature flag schema with cross-reference validation for dependsOn/holdoutId/variantKey. value: yupMixed() for conditions does not enforce that in/not_in operators receive an array, creating a silent always-match/always-block footgun.
apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx PATCH handler switched from atomic override() to explicit get→merge→set, introducing a non-atomic read-modify-write window. PUT now validates config before writing, which is a positive change.
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts Client-side feature flag helpers with session-keyed cache. Cache key is order-sensitive to the input key array, causing unnecessary duplicate requests from components using different orderings.
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/[flagId]/page-client.tsx Flag detail editor with live debug evaluator. Async save/toggle handlers are plain async functions without runAsynchronouslyWithAlert (noted in previous review thread).
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/page-client.tsx Flag list with create dialog. handleCreate now correctly returns prevent-close on failure and shows an alert, fixing the previously flagged silent-close bug.

Sequence Diagram

sequenceDiagram
    participant SDK as Client SDK
    participant Boot as GET /bootstrap
    participant Eval as POST /evaluate
    participant Ev as evaluator.ts
    participant Cfg as Branch Config

    SDK->>Boot: GET bootstrap (If-None-Match: etag)
    Boot->>Cfg: read featureFlags config
    Boot->>Boot: murmur3 hash → version + etag
    alt ETag matches
        Boot-->>SDK: 304 Not Modified
    else Config changed
        Boot-->>SDK: 200 flags + holdouts + version
    end

    SDK->>Eval: POST evaluate with flag_keys
    Eval->>Cfg: read featureFlags config
    alt client auth type
        Eval->>Eval: strip user/team/context/cohorts
        Eval->>Eval: use verified server-side attrs only
    else server or admin auth
        Eval->>Eval: accept caller-supplied targeting context
    end
    Eval->>Ev: evaluateFlag per requested key
    Ev->>Ev: kill_switch, disabled, dep, holdout, rules, default
    Eval-->>SDK: results map with variantKey, value, reason
Loading

Fix All in Claude Code Fix All in Cursor Fix All in Codex

Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
packages/stack-shared/src/config/schema.ts:318-322
**`not_in` / `in` condition values are not type-validated**

The condition `value` is declared as `yupMixed()`, so the schema accepts any JSON value for every operator. For the `not_in` operator specifically, `applyOperator` is:

```ts
case "not_in": { return !(Array.isArray(expected) && expected.includes(actual)); }
```

When `expected` is not an array (e.g. someone types `"admin"` as a plain string instead of `["admin"]`), `Array.isArray(expected)` is `false` so the whole expression becomes `!false === true`, meaning **every user matches the rule unconditionally**. This silently turns an exclusion rule into an "always serve this variant" rule with no error at save time.

The same inverted failure applies to `in`: a non-array value makes it always-false, silently blocking everyone.

Consider adding a `yup.when("operator", …)` refinement that enforces `Array.isArray(value)` when `operator` is `in` or `not_in`.

### Issue 2 of 3
apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx:321-337
**Non-atomic read-modify-write in PATCH handler**

The PATCH route now performs three separate database operations: `get` → (local `override`) → `set`. If two dashboard users push config patches concurrently, both may read the same `oldConfig`, compute independent `newConfig` values, and the second `set` will silently discard whichever changes the first `set` already committed.

The previous implementation delegated to `levelConfig.override()`, which presumably handled the merge atomically (or at a minimum in a single operation). Replacing it with an explicit get/set pair opens a TOCTOU window that didn't exist before. For feature-flag config writes from the dashboard this is unlikely to cause production incidents, but worth noting if atomic writes are achievable here.

### Issue 3 of 3
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts:2692-2695
**`useFeatureFlags` cache key is order-sensitive**

`_serializeFeatureFlagKeys` uses `JSON.stringify(keys)` to derive the cache key. `["a","b"]` and `["b","a"]` stringify differently and produce two independent cache entries, even though the requested flags are identical. A component calling `useFeatureFlags(["checkout","pricing"])` and another calling `useFeatureFlags(["pricing","checkout"])` will each trigger separate network requests and maintain separate subscriptions.

Consider sorting keys before serializing (e.g. `JSON.stringify([...keys].sort())`) so the cache is order-insensitive and components naturally share entries.

Reviews (4): Last reviewed commit: "Enhance feature flag evaluation and conf..." | Re-trigger Greptile

Comment thread apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts
Comment thread apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts Outdated
Comment thread packages/stack-shared/src/feature-flags/evaluator.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Nitpick comments (2)
packages/stack-shared/src/feature-flags/evaluator.ts (1)

18-25: Avoid the cast in the dotted-path lookup helper.

(cursor as Record<string, unknown>) sidesteps the type system in a core helper. You can keep this typed with Reflect.get after the object/null guard instead of casting.

As per coding guidelines "Do NOT use as/any/type casts or anything else like that to bypass the type system. Most of the time a place where you would use type casts is not one where you actually need them".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/stack-shared/src/feature-flags/evaluator.ts` around lines 18 - 25,
The helper getDottedAttribute currently bypasses the type system by casting
cursor to Record<string, unknown>; instead, after the null/object guard inside
the loop use Reflect.get(cursor, part) to access the property without an
explicit cast, preserving proper typing for EvalContext and cursor and returning
undefined when the guard fails; update references to cursor, parts, and
attribute accordingly and remove the (cursor as Record<string, unknown>) cast.
apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts (1)

6-8: Use path notation for the config update helper.

Writing the whole featureFlags object through the plain shape here can overwrite sibling config fields and makes these tests less representative of production updates.

Suggested change
 async function setupProjectWithFlags(featureFlags: FeatureFlagsConfig) {
   await Project.createAndSwitch();
-  await Project.updatePushedConfig({ featureFlags });
+  await Project.updatePushedConfig({ "featureFlags": featureFlags });
 }

As per coding guidelines "When making config updates, use path notation ({ "path.to.field": my-value }) to avoid overwriting sibling properties".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts` around lines 6
- 8, The test currently calls Project.updatePushedConfig with a full nested
object (updatePushedConfig({ featureFlags })) which can overwrite sibling config
fields; change the call in setupProjectWithFlags to use path notation instead
(e.g., Project.updatePushedConfig({ "featureFlags": featureFlags })) so only the
featureFlags path is updated and siblings are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/backend/prisma/seed.ts`:
- Around line 207-210: The "is-employee" rule currently uses operator "contains"
on attribute "user.email", which can match unintended substrings; change it to
an anchored domain match by using a proper operator like "endsWith" with value
"@stack-auth.com" or switch to a regex operator (e.g., "matches") with a pattern
that anchors the domain (such as "@stack-auth\\.com$") so only emails ending in
the company domain are considered internal.

In `@apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts`:
- Around line 35-48: The version hash is computed from JSON.stringify({ flags:
flagsById, flagIdsByKey, holdouts }) which is sensitive to insertion order;
implement a canonicalization step that deep-sorts object keys (and arrays
deterministically) before stringifying so reordering-only changes don't change
the payload; create or use a helper like deepSortObject and call it on { flags:
flagsById, flagIdsByKey, holdouts } prior to passing into
hashingInternal.murmur3_32 to produce the stable version value used by version.

In `@apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts`:
- Around line 36-38: The request schema currently defines cohorts as
yupRecord(yupString(), yupMixed()) and the handler coerces cohort values with
Boolean(...), which incorrectly turns values like "false" into true; change the
schema to validate cohorts as Record<string, boolean> by replacing yupMixed()
with yupBoolean() (use yupRecord(yupString(), yupBoolean()) for the cohorts
field) and remove any Boolean(...) coercion in the handler so the cohorts object
(symbol: cohorts) is passed through unchanged; make the same change for the
other occurrence referenced (lines 58-60) where cohort values are coerced.

In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/feature-flags/[flagId]/page-client.tsx:
- Around line 225-282: The save/delete handlers (handleSaveMetadata,
handleSaveVariants, handleSaveRules, handleDelete) currently always show toast
or navigate regardless of whether updateConfig succeeded; useUpdateConfig
returns a boolean result, so await the updateConfig call, check its returned ok
value, and only call toast or router.push when ok is true; if ok is false,
surface an error (e.g., alert or toast with failure) and do not redirect. Ensure
you reference the updateConfig(...) invocation inside each handler and gate
post-success side effects on the returned boolean.

In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/feature-flags/page-client.tsx:
- Around line 47-55: The current flag factory returns a live-but-unconfigured
flag object (key, type, enabled: true, killSwitch, defaultVariantKey, variants,
rules) which makes non-boolean flags evaluate to undefined; modify the factory
so that when type !== 'boolean' it either sets enabled: false (start as a
disabled draft) or seeds an initial variant and sets defaultVariantKey (e.g.,
create a default entry in variants and assign defaultVariantKey) instead of
leaving defaultVariantKey undefined; update the returned object construction
that builds key/type/enabled/defaultVariantKey/variants to perform this
conditional initialization.

In `@apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts`:
- Around line 11-285: The test suite is missing coverage for two behaviors:
falling back to the authenticated user's id when distinct_id is omitted and
bootstrap revalidation via If-None-Match; add two it tests in this file that use
setupProjectWithFlags and niceBackendFetch: (1) an "/feature-flags/evaluate"
test that authenticates as a user (simulate accessType that ties to an
authenticated user or include an auth header) calls POST without distinct_id and
asserts the returned result reflects that user's id (same deterministic variant
as when you pass distinct_id), and (2) a "/feature-flags/bootstrap" test that
first GETs the bootstrap to capture body.version and then issues a second GET
with an If-None-Match header set to that version and asserts a 304/Not Modified
response (no body) to verify revalidation behavior; place these as additional
it(...) blocks alongside the existing tests referencing niceBackendFetch,
setupProjectWithFlags, and the endpoints "/api/latest/feature-flags/evaluate"
and "/api/latest/feature-flags/bootstrap".

In `@examples/demo/src/app/feature-flags-demo/page.tsx`:
- Around line 15-27: The code silently falls back to empty strings for
NEXT_PUBLIC_STACK_* env vars in evaluateFlags, which masks misconfiguration;
instead, validate apiUrl, projectId, and publishableClientKey at startup (or at
start of evaluateFlags) and throw a clear error if any are missing so the app
fails fast. Replace uses of apiUrl ?? "" / projectId ?? "" /
publishableClientKey ?? "" inside evaluateFlags with the validated non-null
strings (or call a small helper like ensureEnv('NEXT_PUBLIC_STACK_API_URL',
apiUrl)) and propagate/throw a descriptive error if validation fails so the
fetch call never runs with empty/undefined values. Ensure to reference and
update the constants apiUrl, projectId, publishableClientKey and the
evaluateFlags function when making this change.

In `@packages/stack-shared/src/config/schema.ts`:
- Around line 239-291: Add a schema-level test on the feature-flag object (the
schema that contains variants, defaultVariantKey and rules) that collects the
set of variant ids from the variants record and validates that
defaultVariantKey, every rules.*.variantKey, and every key referenced in
rules.*.variantWeights exist in that set; use yup.TestContext (this.createError)
to return clear errors for missing variant ids and reference the existing
symbols: variants, defaultVariantKey, rules, variantKey, variantWeights,
yupRecord and userSpecifiedIdSchema to locate the parent schema; implement the
test as a function that reads this.parent.variants to build the allowed ids,
iterates rules (this.parent.rules) to check each referenced id, and calls
this.createError with appropriate path (e.g. `${this.path}.defaultVariantKey` or
`${this.path}.rules.${ruleId}.variantWeights`) when a missing id is found.

In `@packages/stack-shared/src/feature-flags/evaluator.ts`:
- Around line 205-210: The dependsOn check currently treats only true, non-empty
strings, and non-zero numbers as "met", rejecting JSON objects/arrays; update
evaluateFlag's truthiness logic (the depTruthy computation inside the dependsOn
branch) to use a shared isTruthy helper that returns true for: boolean true,
non-empty strings, non-zero numbers, non-null objects with at least one own key,
non-empty arrays (length > 0); return false for null/undefined/empty
objects/arrays/zero/empty string/false. Replace the inline depTruthy expression
with a call to isTruthy(dep.value) and reuse that helper wherever flag
truthiness is needed (keeping defaultResult("dep_unmet") behavior unchanged).
- Around line 61-67: The evaluator's "regex" branch currently compiles
tenant-supplied patterns on each evaluation (case "regex") which risks ReDoS and
silently swallows pattern errors; change the flow so regex patterns are
validated and pre-compiled at flag creation/update (not in evaluate), store the
compiled/validated RegExp (or a safe-regex wrapper) on the flag object, and in
evaluate use the cached compiled instance's test method without constructing a
new RegExp or relying on a bare catch; also ensure creation/update path rejects
invalid or unsafe patterns (or use a safe-regex library with timeouts) so
evaluate never compiles untrusted input.
- Around line 28-43: The compareSemver implementation is not semver-compliant
because it treats prerelease and build metadata as ordinary sortable tokens;
update compareSemver to parse and compare semver per the spec: split a version
into core (major.minor.patch numeric), prerelease (dot-separated identifiers)
and ignore build metadata entirely, then compare cores numerically, and if equal
treat a version without prerelease as higher than one with prerelease; when
comparing prerelease identifiers apply semver rules (compare numeric identifiers
numerically, non-numeric lexically, numeric < non-numeric, longer list wins only
if all previous equal). Modify the parsing logic in compareSemver and use or
replace stringCompare with the semver identifier comparison rules so that
compareSemver("1.0.0","1.0.0-alpha") > 0 and builds are ignored.

---

Nitpick comments:
In `@apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts`:
- Around line 6-8: The test currently calls Project.updatePushedConfig with a
full nested object (updatePushedConfig({ featureFlags })) which can overwrite
sibling config fields; change the call in setupProjectWithFlags to use path
notation instead (e.g., Project.updatePushedConfig({ "featureFlags":
featureFlags })) so only the featureFlags path is updated and siblings are
preserved.

In `@packages/stack-shared/src/feature-flags/evaluator.ts`:
- Around line 18-25: The helper getDottedAttribute currently bypasses the type
system by casting cursor to Record<string, unknown>; instead, after the
null/object guard inside the loop use Reflect.get(cursor, part) to access the
property without an explicit cast, preserving proper typing for EvalContext and
cursor and returning undefined when the guard fails; update references to
cursor, parts, and attribute accordingly and remove the (cursor as
Record<string, unknown>) cast.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a6f772a3-4cbc-4e0f-9cbc-074953b46d9c

📥 Commits

Reviewing files that changed from the base of the PR and between 65d87a4 and acb1873.

📒 Files selected for processing (18)
  • apps/backend/prisma/seed.ts
  • apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts
  • apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/[flagId]/page-client.tsx
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/[flagId]/page.tsx
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/page-client.tsx
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/page.tsx
  • apps/dashboard/src/lib/apps-frontend.tsx
  • apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts
  • docs/src/components/mdx/app-card.tsx
  • examples/demo/src/app/feature-flags-demo/page.tsx
  • packages/stack-shared/src/apps/apps-config.ts
  • packages/stack-shared/src/config/schema-fuzzer.test.ts
  • packages/stack-shared/src/config/schema.ts
  • packages/stack-shared/src/feature-flags/evaluator.ts
  • packages/stack-shared/src/feature-flags/hashing.ts
  • packages/stack-shared/src/feature-flags/types.ts
  • packages/stack-shared/src/interface/crud/feature-flags.ts

Comment thread apps/backend/prisma/seed.ts Outdated
Comment thread apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts Outdated
Comment thread apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts Outdated
Comment thread examples/demo/src/app/feature-flags-demo/page.tsx
Comment thread packages/stack-shared/src/config/schema.ts
Comment thread packages/stack-shared/src/feature-flags/evaluator.ts Outdated
Comment thread packages/stack-shared/src/feature-flags/evaluator.ts Outdated
Comment thread packages/stack-shared/src/feature-flags/evaluator.ts Outdated
- Updated regex condition handling in feature flags to ensure valid patterns are enforced.
- Enhanced the evaluation logic to handle distinct user IDs and server-side context more effectively.
- Improved response structure for feature flag evaluations, including support for conditional headers and optimized payloads.
- Adjusted the default state of feature flags in the dashboard to be disabled.
- Added comprehensive tests for new evaluation scenarios and response behaviors.
- Implemented `evaluateFeatureFlags` method in `StackClientInterface` to handle feature flag evaluations via the API.
- Added corresponding tests for feature flag evaluation in `client-interface.test.ts` to ensure correct request handling and response validation.
- Introduced new types for feature flag requests and responses in `feature-flags.ts` to standardize data structures.
- Enhanced the `StackClientApp` interface with methods for retrieving single and multiple feature flags, including React hooks for integration.
- Updated documentation to reflect new feature flag functionalities and usage examples.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
packages/stack-shared/src/feature-flags/evaluator.ts (1)

71-81: Consider implementing a size limit or LRU eviction for the regex cache.

The regexCache Map grows indefinitely as new unique patterns are encountered. While this trades memory for repeated regex compilation performance, consider adding a maximum size limit (e.g., LRU eviction) to prevent unbounded memory growth in long-running servers with many distinct patterns. The length validation on individual patterns (maxFeatureFlagRegexPatternLength) caps individual entry size, but doesn't limit the total number of cached entries.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/stack-shared/src/feature-flags/evaluator.ts` around lines 71 - 81,
The regexCache Map in getCompiledRegex grows without bound; implement a bounded
LRU-style eviction. Add a constant (e.g., REGEX_CACHE_MAX = 1000) and update
getCompiledRegex so that on cache hit you promote the key to mark it recent
(delete then set), and on inserting a new compiled RegExp you check
regexCache.size and evict the oldest entry (use regexCache.keys().next().value)
if size > REGEX_CACHE_MAX; alternatively, replace the Map with an LRU cache
library but ensure getCompiledRegex and the regexCache symbol behavior are
preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts`:
- Around line 51-69: The evalContext currently allows client-authenticated
requests to supply distinct_id (via the else branch using body.distinct_id),
enabling client-side spoofing; update the else/unauthorized-targeting branch so
distinctId is taken only from auth.user?.id (remove body.distinct_id) and ensure
userId and user fields likewise come from auth (e.g., userId: auth.user?.id and
user built from auth.user), preserving the existing
callerCanSupplyTargetingContext check and leaving the auth.type !== "client"
branch unchanged.

In `@packages/stack-shared/src/feature-flags/types.ts`:
- Around line 25-40: Replace the fragile, hand-rolled ReDoS checks in
getFeatureFlagRegexPatternError with a proven ReDoS-safe engine or validator:
remove or keep only the simple length check (maxFeatureFlagRegexPatternLength)
and instead use a library like re2 or safe-regex to validate the pattern (call
the library from getFeatureFlagRegexPatternError before constructing new RegExp)
so you guarantee linear-time matching; ensure the function returns the
library-provided error message on invalid patterns and still returns undefined
for valid ones.

In `@packages/stack-shared/src/interface/client-interface.ts`:
- Around line 552-568: The evaluateFeatureFlags method currently calls
sendClientRequest without passing the requestType so it always defaults to
client access; update evaluateFeatureFlags to accept a requestType parameter (or
forward an existing one) and pass it as the fourth argument to sendClientRequest
so the X-Stack-Access-Type header reflects the caller (e.g., call
sendClientRequest(..., session, requestType)). Ensure you update the
evaluateFeatureFlags signature and any call sites to supply the appropriate
requestType and preserve existing behavior when omitted.

---

Nitpick comments:
In `@packages/stack-shared/src/feature-flags/evaluator.ts`:
- Around line 71-81: The regexCache Map in getCompiledRegex grows without bound;
implement a bounded LRU-style eviction. Add a constant (e.g., REGEX_CACHE_MAX =
1000) and update getCompiledRegex so that on cache hit you promote the key to
mark it recent (delete then set), and on inserting a new compiled RegExp you
check regexCache.size and evict the oldest entry (use
regexCache.keys().next().value) if size > REGEX_CACHE_MAX; alternatively,
replace the Map with an LRU cache library but ensure getCompiledRegex and the
regexCache symbol behavior are preserved.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 375a444a-d336-4e88-a7aa-32d868fd97af

📥 Commits

Reviewing files that changed from the base of the PR and between acb1873 and 9956903.

📒 Files selected for processing (22)
  • apps/backend/prisma/seed.ts
  • apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts
  • apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts
  • apps/backend/src/route-handlers/smart-response.tsx
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/[flagId]/page-client.tsx
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/page-client.tsx
  • apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts
  • packages/stack-shared/src/config/schema.ts
  • packages/stack-shared/src/feature-flags/evaluator.ts
  • packages/stack-shared/src/feature-flags/hashing.ts
  • packages/stack-shared/src/feature-flags/types.ts
  • packages/stack-shared/src/interface/client-interface.test.ts
  • packages/stack-shared/src/interface/client-interface.ts
  • packages/stack-shared/src/interface/crud/feature-flags.ts
  • packages/template/src/index.ts
  • packages/template/src/lib/hooks.tsx
  • packages/template/src/lib/stack-app/apps/implementations/client-app-feature-flags.test.tsx
  • packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts
  • packages/template/src/lib/stack-app/apps/index.ts
  • packages/template/src/lib/stack-app/apps/interfaces/client-app.ts
  • packages/template/src/lib/stack-app/index.ts
  • sdks/spec/src/apps/client-app.spec.md
✅ Files skipped from review due to trivial changes (3)
  • packages/template/src/lib/stack-app/apps/index.ts
  • packages/template/src/lib/stack-app/index.ts
  • packages/stack-shared/src/interface/client-interface.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • apps/backend/prisma/seed.ts
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/page-client.tsx
  • packages/stack-shared/src/interface/crud/feature-flags.ts
  • apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts
  • apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts

Comment thread apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts
Comment thread packages/stack-shared/src/config/schema.ts
Comment thread packages/stack-shared/src/feature-flags/types.ts
Comment thread packages/stack-shared/src/interface/client-interface.ts
Comment thread apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts
- Updated package versions in pnpm-lock.yaml, including `react-dom` and `lightningcss`.
- Added new dependencies for the dashboard application, including various UI and utility libraries.
- Enhanced the `GET` route for feature flags to support additional response types and improved validation.
- Refactored the `POST` route to streamline user ID handling and ensure consistent behavior.
- Updated tests to reflect changes in API behavior and ensure proper request handling.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts`:
- Around line 74-83: The current If-None-Match check treats
headers["if-none-match"] as a single string and calls includes(etag), which
misses comma-separated ETag lists; update the comparison in the route so you
parse headers["if-none-match"] into individual entity tags (split on commas,
trim whitespace and surrounding quotes, and handle the wildcard "*" case) and
then check if any parsed tag equals the current etag or version before returning
the 304 response; locate the logic around the if block that references
headers["if-none-match"], etag and version to implement this parsing and
comparison.

In `@apps/backend/src/app/api/latest/internal/config/override/`[level]/route.tsx:
- Around line 301-317: The current flow reads the entire override
(levelConfig.get), merges with override(), validates, then writes the whole
document (levelConfig.set), which is not atomic and can silently clobber
concurrent sibling updates; instead build a path-based partial update from
parsedConfig and apply it with a path/patch API on levelConfig (e.g., use the
service's patch/update-paths method or a set call that accepts dotted-path keys)
rather than reading oldConfig and calling levelConfig.set with the merged
document; keep using assertConfigValidBeforeWrite but validate the specific
path(s) or the resulting merged paths, then call the path-based update
(referencing levelConfig.get, override, assertConfigValidBeforeWrite,
levelConfig.set in your changes) so concurrent updates don't overwrite unrelated
properties.

In `@packages/stack-shared/src/feature-flags/evaluator.ts`:
- Around line 28-45: compareSemver currently falls back to lexicalCompare for
malformed versions which lets bad inputs pass semver_gt/semver_lt; update parse
inside compareSemver so it performs strict validation (core must be three
numeric segments and prerelease parsed deterministically) and if either operand
is invalid return NaN (or another sentinel) instead of calling lexicalCompare,
then update semver_gt and semver_lt to call compareSemver and return false when
the result is NaN (i.e., only return true when compareSemver yields a valid
numeric comparison); ensure you remove the lexicalCompare fallback and apply the
same validation change where compareSemver is duplicated/used around lines
149-158.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 251d880d-ffb7-44d8-adec-444271adce91

📥 Commits

Reviewing files that changed from the base of the PR and between 9956903 and f053ad2.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (9)
  • apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts
  • apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts
  • apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx
  • apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts
  • packages/stack-shared/package.json
  • packages/stack-shared/src/feature-flags/evaluator.ts
  • packages/stack-shared/src/feature-flags/types.ts
  • packages/stack-shared/src/interface/client-interface.test.ts
  • packages/stack-shared/src/interface/client-interface.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/stack-shared/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
  • packages/stack-shared/src/interface/client-interface.ts
  • packages/stack-shared/src/interface/client-interface.test.ts
  • apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts
  • apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts

Comment thread apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts Outdated
Comment thread packages/stack-shared/src/feature-flags/evaluator.ts Outdated
- Added a `hidden` metadata property to the `GET` route for feature flags to control visibility.
- Refactored the `if-none-match` header handling in the `GET` route to improve cache validation using a new `parseIfNoneMatch` function.
- Updated the `PATCH` route for configuration overrides to streamline the process by directly using the new override methods for branch, environment, and project configurations.
- Improved user feedback in the dashboard by changing the alert message for failed feature flag creation attempts.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (5)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/page-client.tsx (3)

69-70: Add defensive fallback for potentially undefined flags.

If config.featureFlags.flags is undefined (e.g., a freshly initialized project), passing undefined to typedEntries could behave unexpectedly. A defensive fallback keeps the component robust.

🛡️ Proposed fix
-  const flags = config.featureFlags.flags;
+  const flags = config.featureFlags.flags ?? {};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/feature-flags/page-client.tsx
around lines 69 - 70, config.featureFlags.flags may be undefined, so calling
typedEntries(flags) can break; add a defensive fallback by ensuring flags is an
object/empty map before calling typedEntries (e.g., set flags =
config.featureFlags.flags ?? {}), then compute entries = typedEntries(flags) so
typedEntries always receives a defined value; update the code around the
variables flags and entries in page-client.tsx to use this fallback.

140-142: Consider using ?? for consistency with other nullish checks.

Line 140 uses || while line 142 uses ??. With ||, an empty string "" for def.key would fall back to flagId, whereas ?? only falls back for null/undefined. If empty keys shouldn't exist (enforced by validation), using ?? maintains consistent semantics. As per coding guidelines, prefer explicit null/undefined checks over boolean checks.

♻️ Proposed fix
-                title={def.key || flagId}
+                title={def.key ?? flagId}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/feature-flags/page-client.tsx
around lines 140 - 142, The title prop currently uses a boolean fallback
(title={def.key || flagId}) which treats empty string as absent; change it to a
nullish fallback (title={def.key ?? flagId}) to match the existing nullish check
used for subtitle (def.type ?? "boolean") and keep semantics consistent for
def.key and flagId; locate the title assignment in the component rendering where
title, subtitle are set and replace the || operator with ?? for def.key.

181-181: Replace type cast with a type guard for safer type narrowing.

The as FlagType cast bypasses type safety. A type guard validates the value at runtime and satisfies the type system without an unsafe cast. As per coding guidelines, avoid using as to bypass the type system.

♻️ Proposed fix

Add a helper near the FlagType definition:

const FLAG_TYPES = new Set<FlagType>(["boolean", "multivariate", "json", "numeric", "string"]);

function isFlagType(v: string): v is FlagType {
  return FLAG_TYPES.has(v as FlagType);
}

Then use it in the handler:

-              <Select value={newType} onValueChange={(v) => setNewType(v as FlagType)}>
+              <Select value={newType} onValueChange={(v) => { if (isFlagType(v)) setNewType(v); }}>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/feature-flags/page-client.tsx
at line 181, Replace the unsafe cast in the Select onValueChange handler by
adding a runtime type guard for FlagType: define FLAG_TYPES (Set of valid
FlagType strings) and an isFlagType(v: string): v is FlagType function, then
change the handler in Select (the component using newType and setNewType) to
call isFlagType(v) and only call setNewType(v) when the guard returns true; this
removes the use of "as FlagType" and preserves type safety for newType and
setNewType.
apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx (2)

260-264: Avoid double-validating the same config in PUT.

You validate in assertConfigValidBeforeWrite (Line 260) and then immediately validate again in warnOnValidationFailure (Line 273) with the same payload. Consider removing or gating the second call to reduce redundant work and transient log noise.

Also applies to: 273-277

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/app/api/latest/internal/config/override/`[level]/route.tsx
around lines 260 - 264, The PUT handler is running duplicate validation:
assertConfigValidBeforeWrite(levelConfig, {projectId,branchId,config:
parsedConfig}) and then immediately calling warnOnValidationFailure with the
same payload; remove the redundant work by reusing the first validation result
or gating the second call—either have assertConfigValidBeforeWrite return its
validation result and pass that into warnOnValidationFailure (so
warnOnValidationFailure no longer re-runs validation), or only call
warnOnValidationFailure when assertConfigValidBeforeWrite completes successfully
(avoiding a second validation run). Ensure you update references to levelConfig,
parsedConfig, assertConfigValidBeforeWrite, and warnOnValidationFailure
accordingly.

59-62: Add inline comments explaining the any types, or refactor to use schema-derived types.

The three instances of config: any in the validate and helper function signatures (lines 59, 89, and 215) lack justification. Per coding guidelines, use yup.InferType<typeof schema> or add a comment explaining why any is necessary here (e.g., because the config undergoes runtime validation and normalization with complex polymorphic transforms).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/app/api/latest/internal/config/override/`[level]/route.tsx
around lines 59 - 62, The parameter types currently using `any` (e.g., the
`options.config` in the validate callback and the helper functions like
validateProjectConfigOverride and the other helper at line ~215) should be
replaced with schema-derived types or documented why `any` is required: change
`any` to `yup.InferType<typeof <YourSchemaName>>` (e.g.,
ProjectConfigOverrideSchema) so the validateProjectConfigOverride signature
becomes typed from the schema, or if you cannot due to complex polymorphic
transforms, add a concise inline comment above each function explaining that
runtime validation/normalization makes static typing impractical and why `any`
is used temporarily (include reference to the specific schema used). Ensure you
update all three spots (the validate callback, validateProjectConfigOverride,
and the helper at ~215) to use the same approach and reference the schema symbol
name to keep types consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/stack-shared/src/feature-flags/evaluator.ts`:
- Around line 150-158: The toMs helper in the "before"/"after" branch currently
uses Date.parse on arbitrary strings which is implementation-dependent; update
the toMs function (used in the "before"/"after" case) to fail closed for string
inputs by only accepting Date objects or numeric epoch values, or if you must
support strings validate against a strict RFC3339/ISO8601 pattern first and only
then parse; return undefined for any non-matching strings so comparisons are
deterministic across runtimes.
- Around line 18-25: getDottedAttribute currently uses Reflect.get which walks
the prototype chain and can return inherited properties; change it to only read
own properties: when iterating parts inside getDottedAttribute, after confirming
cursor is non-null and typeof object, check
Object.prototype.hasOwnProperty.call(cursor, part) (or
Object.getOwnPropertyDescriptor) and if the property is not an own property
return undefined; if it is own, read the value via direct property access (e.g.,
(cursor as Record<string, unknown>)[part]) and continue. This ensures
getDottedAttribute resolves only explicitly provided fields and not inherited
ones.

---

Nitpick comments:
In `@apps/backend/src/app/api/latest/internal/config/override/`[level]/route.tsx:
- Around line 260-264: The PUT handler is running duplicate validation:
assertConfigValidBeforeWrite(levelConfig, {projectId,branchId,config:
parsedConfig}) and then immediately calling warnOnValidationFailure with the
same payload; remove the redundant work by reusing the first validation result
or gating the second call—either have assertConfigValidBeforeWrite return its
validation result and pass that into warnOnValidationFailure (so
warnOnValidationFailure no longer re-runs validation), or only call
warnOnValidationFailure when assertConfigValidBeforeWrite completes successfully
(avoiding a second validation run). Ensure you update references to levelConfig,
parsedConfig, assertConfigValidBeforeWrite, and warnOnValidationFailure
accordingly.
- Around line 59-62: The parameter types currently using `any` (e.g., the
`options.config` in the validate callback and the helper functions like
validateProjectConfigOverride and the other helper at line ~215) should be
replaced with schema-derived types or documented why `any` is required: change
`any` to `yup.InferType<typeof <YourSchemaName>>` (e.g.,
ProjectConfigOverrideSchema) so the validateProjectConfigOverride signature
becomes typed from the schema, or if you cannot due to complex polymorphic
transforms, add a concise inline comment above each function explaining that
runtime validation/normalization makes static typing impractical and why `any`
is used temporarily (include reference to the specific schema used). Ensure you
update all three spots (the validate callback, validateProjectConfigOverride,
and the helper at ~215) to use the same approach and reference the schema symbol
name to keep types consistent.

In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/feature-flags/page-client.tsx:
- Around line 69-70: config.featureFlags.flags may be undefined, so calling
typedEntries(flags) can break; add a defensive fallback by ensuring flags is an
object/empty map before calling typedEntries (e.g., set flags =
config.featureFlags.flags ?? {}), then compute entries = typedEntries(flags) so
typedEntries always receives a defined value; update the code around the
variables flags and entries in page-client.tsx to use this fallback.
- Around line 140-142: The title prop currently uses a boolean fallback
(title={def.key || flagId}) which treats empty string as absent; change it to a
nullish fallback (title={def.key ?? flagId}) to match the existing nullish check
used for subtitle (def.type ?? "boolean") and keep semantics consistent for
def.key and flagId; locate the title assignment in the component rendering where
title, subtitle are set and replace the || operator with ?? for def.key.
- Line 181: Replace the unsafe cast in the Select onValueChange handler by
adding a runtime type guard for FlagType: define FLAG_TYPES (Set of valid
FlagType strings) and an isFlagType(v: string): v is FlagType function, then
change the handler in Select (the component using newType and setNewType) to
call isFlagType(v) and only call setNewType(v) when the guard returns true; this
removes the use of "as FlagType" and preserves type safety for newType and
setNewType.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ff8cce1b-8187-41f7-ba38-dfea16d3f590

📥 Commits

Reviewing files that changed from the base of the PR and between f053ad2 and 3fcff8b.

📒 Files selected for processing (4)
  • apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts
  • apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/page-client.tsx
  • packages/stack-shared/src/feature-flags/evaluator.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/backend/src/app/api/latest/feature-flags/bootstrap/route.ts

Comment thread packages/stack-shared/src/feature-flags/evaluator.ts
Comment on lines +150 to +158
case "before":
case "after": {
const toMs = (v: unknown) => {
if (v instanceof Date) return v.getTime();
if (typeof v === "number") return v;
if (typeof v === "string") {
const ms = Date.parse(v);
return Number.isNaN(ms) ? undefined : ms;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Date.parse() determinism Node.js vs browser JavaScript differences implementation-defined

💡 Result:

Date.parse is not deterministic across JavaScript environments like Node.js and browsers due to its implementation-defined behavior for non-standard date string formats. The ECMAScript specification mandates parsing only for a specific simplified ISO 8601 format (e.g., "YYYY-MM-DDTHH:mm:ss.sssZ"). All other formats may use engine-specific heuristics, leading to different results or Invalid Date across engines. Key differences: - Both Node.js (V8 engine) and Chrome (V8) share the same lenient parser, often accepting ambiguous formats like MM/DD/YYYY as local time or handling edge cases (e.g., "2015/03/31" as local midnight vs "2015-03-31" as UTC midnight). - Firefox (SpiderMonkey) may reject more strings as Invalid Date or interpret differently (e.g., stricter on day-first formats like "29/06/2024"). - Safari has historically varied, sometimes rejecting space-separated ISO-like strings. - Examples: new Date("2024-06-29 14:30:00") (space instead of T) works in Chrome/Node.js/Firefox as local but may fail in older Safari. new Date("06/29/2024") is US-local in most but DD/MM/YYYY in some older Firefox. Node.js vs browsers: Minimal differences since Node.js uses V8 like Chrome. Subtle variations can arise from system timezone settings or V8 version differences, but primarily engine-driven. Pre-ES6, more variance existed; ES6+ standardized some ISO handling (absent offset = local time). Avoid Date.parse for non-ISO; use manual parsing, Date.UTC, or libraries like date-fns/Luxon. Temporal API (ES2026+) offers deterministic alternatives.

Citations:


🌐 Web query:

JavaScript Date.parse RFC 3339 ISO 8601 format Node.js browser compatibility implementation-defined

💡 Result:

JavaScript's Date.parse has implementation-defined behavior for most date string formats, including those beyond the strict ECMAScript "Date Time String Format" (a profile of ISO 8601 extended format similar to RFC 3339). Only strings exactly matching this format are required to parse consistently across engines: YYYY-MM-DDTHH:mm:ss.sssZ (with optional reduced precision, optional ±HH:mm offset instead of Z). RFC 3339 and full ISO 8601 are broader; Date.parse must accept the spec format but may fall back to engine-specific parsing for others, leading to inconsistencies. Examples: - Date-only YYYY-MM-DD parses as UTC (spec requirement, differs from ISO 8601 local interpretation). - Variations like lowercase 't/z', out-of-bounds days (e.g., "2018-02-30"), extra fraction digits, or missing colons in offsets often parse differently: Chrome/Edge/Safari may normalize/fix, Firefox rejects more strictly. Browser compatibility (ISO 8601 basic support since ~2010-2015, full across modern browsers): - Chrome 6+, Firefox 4+, Safari 5.1+, Edge 12+, Opera 10+. Node.js (V8-based) matches Chrome behavior. No changes in ES2026+; Temporal API introduced as reliable alternative, but Date.parse remains implementation-defined. For cross-environment safety, use strict spec format or libraries/Temporal. Manual parsing recommended for RFC 3339 edge cases.

Citations:


Make date comparisons use a deterministic string format.

Date.parse() accepts implementation-defined formats beyond ISO 8601. Because this evaluator runs in both the backend and SDKs, a non-ISO input can produce different "before" / "after" results across runtimes. Please fail closed unless the string is in a strict format (such as RFC 3339/ISO 8601), or restrict this operator to Date / epoch-number inputs only.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/stack-shared/src/feature-flags/evaluator.ts` around lines 150 - 158,
The toMs helper in the "before"/"after" branch currently uses Date.parse on
arbitrary strings which is implementation-dependent; update the toMs function
(used in the "before"/"after" case) to fail closed for string inputs by only
accepting Date objects or numeric epoch values, or if you must support strings
validate against a strict RFC3339/ISO8601 pattern first and only then parse;
return undefined for any non-matching strings so comparisons are deterministic
across runtimes.

- Set React and React-DOM to version 19.2.3 in package.json.
- Updated pnpm-lock.yaml to reflect the new versions for React and React-DOM, ensuring consistency across dependencies.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@package.json`:
- Around line 101-102: package.json pins runtime "react" and "react-dom" to
19.2.3 while the workspace override forces `@types/react` and `@types/react-dom` to
^18.2.0, causing a major-version mismatch; update the workspace overrides so the
dev/type packages match React 19 (e.g., set `@types/react` and `@types/react-dom` to
the correct major for React 19), or alternatively align the runtime to React 18
if you must keep the existing types—modify the overrides entry for
"@types/react" and "@types/react-dom" (and any related peerDependency entries)
to the matching major version to ensure type/runtime parity across the
workspace.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: be85bd6e-4487-4230-9d4f-3dc8e9747dfc

📥 Commits

Reviewing files that changed from the base of the PR and between 3fcff8b and 45494e3.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (1)
  • package.json

Comment thread package.json Outdated
Comment on lines +101 to +102
"react": "19.2.3",
"react-dom": "19.2.3",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python - <<'PY'
import json, pathlib

root = json.loads(pathlib.Path("package.json").read_text())
print("Root versions:")
print("  devDependencies.react =", root.get("devDependencies", {}).get("react"))
print("  devDependencies.react-dom =", root.get("devDependencies", {}).get("react-dom"))
print("  pnpm.overrides.@types/react =", root.get("pnpm", {}).get("overrides", {}).get("@types/react"))
print("  pnpm.overrides.@types/react-dom =", root.get("pnpm", {}).get("overrides", {}).get("@types/react-dom"))

print("\nWorkspace package.json files with React/type entries:")
for p in pathlib.Path(".").rglob("package.json"):
    if "node_modules" in p.parts:
        continue
    data = json.loads(p.read_text())
    found = []
    for section in ("dependencies", "devDependencies", "peerDependencies", "optionalDependencies"):
        deps = data.get(section, {})
        for key in ("react", "react-dom", "@types/react", "@types/react-dom"):
            if key in deps:
                found.append((section, key, deps[key]))
    if found:
        print(f"- {p}")
        for section, key, val in found:
            print(f"    {section}.{key} = {val}")
PY

Repository: stack-auth/stack-auth

Length of output: 5396


Align React runtime and React type majors.

Lines 101-102 pin runtime to React 19.2.3, but lines 113-114 override @types/react and @types/react-dom to ^18.2.0. This major version mismatch between runtime and types can break or hide React 19 typings across the entire workspace, even for packages that declare higher type versions locally, since pnpm overrides take precedence workspace-wide.

Suggested fix
   "pnpm": {
     "overrides": {
-      "@types/react": "^18.2.0",
-      "@types/react-dom": "^18.2.0"
+      "@types/react": "^19.0.0",
+      "@types/react-dom": "^19.0.0"
     },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 101 - 102, package.json pins runtime "react" and
"react-dom" to 19.2.3 while the workspace override forces `@types/react` and
`@types/react-dom` to ^18.2.0, causing a major-version mismatch; update the
workspace overrides so the dev/type packages match React 19 (e.g., set
`@types/react` and `@types/react-dom` to the correct major for React 19), or
alternatively align the runtime to React 18 if you must keep the existing
types—modify the overrides entry for "@types/react" and "@types/react-dom" (and
any related peerDependency entries) to the matching major version to ensure
type/runtime parity across the workspace.

@mantrakp04 mantrakp04 assigned mantrakp04 and unassigned mantrakp04 Apr 29, 2026
@mantrakp04 mantrakp04 added codex and removed codex labels Apr 29, 2026
@mantrakp04 mantrakp04 requested review from BilalG1 and removed request for BilalG1 April 29, 2026 03:03
…ck.yaml

- Deleted React and React-DOM version specifications from package.json.
- Updated pnpm-lock.yaml to remove references to React and React-DOM, ensuring a clean dependency list.
- Introduced verification for primary email in the feature flag evaluation context to ensure only verified emails are used.
- Updated the evaluation logic to handle distinct IDs more flexibly, allowing for better fallback mechanisms.
- Improved the PATCH route for configuration overrides by implementing a new method to merge old and new configurations, ensuring validation before writing.
- Added comprehensive tests to validate the new feature flag evaluation scenarios, including handling of unverified emails and dependency cycles.
Comment on lines +318 to +322
return this.createError({
message: "Feature flag regex conditions must use a string value",
path: `${this.path}.value`,
});
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 not_in / in condition values are not type-validated

The condition value is declared as yupMixed(), so the schema accepts any JSON value for every operator. For the not_in operator specifically, applyOperator is:

case "not_in": { return !(Array.isArray(expected) && expected.includes(actual)); }

When expected is not an array (e.g. someone types "admin" as a plain string instead of ["admin"]), Array.isArray(expected) is false so the whole expression becomes !false === true, meaning every user matches the rule unconditionally. This silently turns an exclusion rule into an "always serve this variant" rule with no error at save time.

The same inverted failure applies to in: a non-array value makes it always-false, silently blocking everyone.

Consider adding a yup.when("operator", …) refinement that enforces Array.isArray(value) when operator is in or not_in.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/stack-shared/src/config/schema.ts
Line: 318-322

Comment:
**`not_in` / `in` condition values are not type-validated**

The condition `value` is declared as `yupMixed()`, so the schema accepts any JSON value for every operator. For the `not_in` operator specifically, `applyOperator` is:

```ts
case "not_in": { return !(Array.isArray(expected) && expected.includes(actual)); }
```

When `expected` is not an array (e.g. someone types `"admin"` as a plain string instead of `["admin"]`), `Array.isArray(expected)` is `false` so the whole expression becomes `!false === true`, meaning **every user matches the rule unconditionally**. This silently turns an exclusion rule into an "always serve this variant" rule with no error at save time.

The same inverted failure applies to `in`: a non-array value makes it always-false, silently blocking everyone.

Consider adding a `yup.when("operator", …)` refinement that enforces `Array.isArray(value)` when `operator` is `in` or `not_in`.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Cursor Fix in Codex

@mantrakp04 mantrakp04 requested a review from BilalG1 May 1, 2026 16:45
@mantrakp04 mantrakp04 marked this pull request as draft May 4, 2026 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants